Add title feature to legend and support legend option for various traces #4386
Conversation
- choropleth - choroplethmapbox - cone - densitymapbox - heatmap - histogram2d - isosurface - mesh3d - streamtube - surface - volume
| trace._module && | ||
| trace._module.attributes && | ||
| trace._module.attributes.showlegend && | ||
| trace._module.attributes.showlegend.dflt === false |
There was a problem hiding this comment.
Can you explain why we need this extra piece of logic:
!(
trace._module &&
trace._module.attributes &&
trace._module.attributes.showlegend &&
trace._module.attributes.showlegend.dflt === falsehere?
There was a problem hiding this comment.
Good question.
This is to keep the baselines to what was in place before.
Otherwise if there are e.g. one scatter3d with other gl3d traces (that now support legend) the legend would show up on the graph for scatter3d.
src/components/legend/draw.js
Outdated
| var opts = gd._fullLayout.legend; | ||
| var bw = gd._fullLayout.legend.borderwidth; | ||
| var opts = legendItem ? | ||
| gd._fullLayout.legend : | ||
| gd._fullLayout.legend.title; |
There was a problem hiding this comment.
I'm not a big fan of making computeTextDimensions work for the legend items AND the legend title.
What if you revert computeTextDimensions to the way it was and added a similar (but different enough) computeTitleDimensions?
There was a problem hiding this comment.
I don't like the complexities you brought into computeTextDimensions, but I don't don't like them enough to block this PR.
There was a problem hiding this comment.
... so don't be surprised if I change the computeTextDimensions pattern in a future legend PR of mine.
There was a problem hiding this comment.
Sure. Please go for it.
| .enter().append('g') | ||
| .classed('legendpoints', true); | ||
| }) | ||
| .each(styleSpatial) |
There was a problem hiding this comment.
[Do not address this comment in this PR - I'm just writing down ideas for future development]
I think legend/style.js deserves a refactor. Instead of having the styling logic for all the traces that support legends in here, we should implement a new trace module method named e.g. _module.legendItem. This would be more inline with how colorbar styling works. legend/style.js would then be pretty trivial: a few legend-wide .attr() and then a loop over the items for-each calling their corresponding _module.legendItem method. Moreover, by setting a _module.legendItem method for all the traces that support legend items, we wouldn't need the 'showLegend' trace module category.
| case 'histogram2d' : | ||
| case 'heatmap' : | ||
| ptsData = [ | ||
| ['M-15,-2V4H15V-2Z'] // similar to contour |
There was a problem hiding this comment.
Ha you went ahead and made the heatmap legend items span rectangles:
instead of squares. I like it, but it might be nice to get a few +1s from other folks.
src/components/legend/draw.js
Outdated
| .call(Drawing.font, title.font) | ||
| .text(title.text); | ||
|
|
||
| textLayout(titleEl, legend, gd); // handle mathjax or multi-line text and compute title height |
There was a problem hiding this comment.
Nice!! Looks like you got MathJax to work:
| "name": "Canada", | ||
| "locations": ["CAN"], | ||
| "z": [1], | ||
| "colorscale": [[0, "blue"], [1, "blue"]], |
|
Thanks @archmoj for taking this on!! Looks like you're not too far off from ✅ the two items 🚀 I noticed that you didn't add any After this PR, if I counted right, the trace types that have legend support are:
and the trace types that do not have legend support are:
|
|
This all looks great to me! The list of traces without legend support matches my expectations! Notes:
|
- stash _titleWidth and _titleHeight once in _fullLayout.legend - bring back test for prune unsupported global-level test and use parcoords instead of choropleth - reuse getGradientDirection function to handle reversescale case - reserve new lines for Lib.coerce argumnets not ternary operator - add a jasmine test for legend.title.side relayout
|
Do all the traces that now have legend entries therefore get |
Yes. |
|
Nicely done @archmoj !! 💃 💃 |
Fix image exports for new #4386 mocks with SVG gradients
3 participants
resolves #276 and
resolves #2642 and
resolves #3468
List of new
layoutattributes:legend.titlelegend.title.textlegend.title.fontlegend.title.sideList of new traces with
legend:choroplethchoroplethmapboxconedensitymapboxheatmaphistogram2disosurfacemesh3dstreamtubesurfacevolume@plotly/plotly_js
cc: @nicolaskruchten
before vs after demo